fix: mender-inventory-provides to read the device_type from mender.conf #1701
Conversation
|
@MuchoLucho, Let me know if you want to start the integration pipeline by mentioning me and the command "start pipeline". my commands and optionsYou can trigger a pipeline on multiple prs with:
You can start a fast pipeline, disabling full integration tests with:
You can trigger GitHub->GitLab branch sync with:
You can cherry pick to a given branch or branches with:
|
MuchoLucho
force-pushed
the
fix-inventory-provides
branch
2 times, most recently
from
ed0839d to
5f8bc8d
Compare
support/mender-inventory-provides
Outdated
| default_device_type_file="/var/lib/mender/device_type" | ||
|
|
||
| # Read mender.conf file to find the DeviceTypeFile key if it exists prioritizing /etc over /var/lib | ||
| if grep -q '"DeviceTypeFile"' /etc/mender/mender.conf; then |
There was a problem hiding this comment.
/etc/mender/mender.conf
This can be influenced by env variables and flags
mender-update -h
NAME:
mender-update - manage and start Mender Update
...
...
Environment variables:
- MENDER_CONF_DIR - configuration (default: /etc/mender).
- MENDER_DATA_DIR - identity, inventory and update modules (default: /usr/share/mender).
- MENDER_DATASTORE_DIR - runtime datastore (default: /var/lib/mender).
COMMANDS:
...
GLOBAL OPTIONS:
--config FILE, -c FILE Configuration FILE path (default:
/etc/mender/mender.conf)
--fallback-config FILE, -b FILE Fallback configuration FILE path (default:
/var/lib/mender/mender.conf)
Don't have an idea on top of my head on how to approach this.
Just a note that the the hardcoded mender.conf may become the new /var/lib/mender/device_type :)
There was a problem hiding this comment.
I assumed that despite you change the env variables, the /etc/mender/mender.cond and /var/lib/mender/mender.conf will not. I know it doesn't make that much sense, but otherwise we need to also alter the first three paragraphs as part of a parallel PR: https://docs.mender.io/client-installation/configuration#configuration
What do you think @TheMeaningfulEngineer ?
There was a problem hiding this comment.
@TheMeaningfulEngineer what do you think about bff1a4f?
I don't know how to handle if mender-update was executed with any of the follow ones:
--config FILE, -c FILE
--fallback-config FILE, -b FILE
--data DIR, -d DIR
Any advise @lluiscampos?
There was a problem hiding this comment.
We cannot possibly get the command line options of mender-auth or mender-update from the script. It is correct to use the environment variables (good call on that too!).
In a way the more we align this code with the rootfs-image the better.
There was a problem hiding this comment.
Looking good!
Few things:
- Squash your three commits into one.
- Two of your commits have a typo
Title: Noneinstead ofTicket: None😅
- Two of your commits have a typo
- Please consider a more user friendly changelog entry, this is a bug after all. Example: "Fix inventory reporting of
device_typeto correctly select the file based on Mender configuration and Mender environment variables" - See also nitpick below 👇
|
@MuchoLucho Can you attend this one? We are going to be releasing very soon and this is a good fix to have. I can take over the work if you want. |
MuchoLucho
force-pushed
the
fix-inventory-provides
branch
from
bff1a4f to
95d7fd7
Compare
support/mender-inventory-provides
Outdated
| device_type_file=$(sed -ne '/"'"$match"'" *: *"[^"]*"/ { s/.*"'"$match"'" *: *"\([^"]*\)".*/\1/; p }' "$mender_conf_dir/mender.conf" || true) | ||
| fi | ||
|
|
||
| if [ -z "$device_type_file" ] && [ -f "$mender_datastore_dir/mender.conf" ]; then |
There was a problem hiding this comment.
If $device_type_file variable is defined (iow got a value from /etc/mender/mender.conf) but the device_type file does not exist, we'll not get here for the fallback case where a different file could be specified and could be valid.
I don't know how important this is to handle, though. Is just an observation while experimenting with your fix on my device.
Edit: to make clear my comment - it could be handled with something like:
if [ -f "$mender_datastore_dir/mender.conf" ]; then
if [ -z "$device_type_file" ] || [ ! -f "$device_type_file" ]; then
...
fi
fi
…ic from the Mender configuration file - Add support for reading the paths from env variables `MENDER_CONF_DIR` and `MENDER_DATASTORE_DIR` when relocating the device_type file. - Implement configuration parsing to extract `DeviceTypeFile` from `mender.conf` despite its location. - Fallback to the default `device_type` file if `DeviceTypeFile` is not set or the extracted file path does not exist. Changelog: Fix inventory reporting of device_type to correctly select the file based on Mender configuration and Mender environment variables Ticket: None Signed-off-by: Luis Ramirez <[email protected]> Signed-off-by: Lluis Campos <[email protected]>
lluiscampos
force-pushed
the
fix-inventory-provides
branch
from
95d7fd7 to
adf65a9
Compare
|
Merging these commits will result in the following changelog entries: Changelogsmender (fix-inventory-provides)New changes in mender since master: Bug Fixes
|
|
@mender-test-bot start pipeline |
|
Hello 😺 I created a pipeline for you here: Pipeline-1586226025 Build Configuration Matrix
|
|
Hello 😺 This PR contains changelog entries. Please, verify the need of backporting it to the following release branches: |
|
@mender-test-bot cherry-pick to:
|
|
Hi 😺 |
Title: None
Ticket: None
External Contributor Checklist
🚨 Please review the guidelines for contributing to this repository.
The majority of our contributions are fixes, which means your commit should have
the form below:
git --signoff. Also note that the signoff author must match the author of the commit.Description
Please describe your pull request.
Thank you!